Remove SuperHeaders class, add new from header methods#10911
Remove SuperHeaders class, add new from header methods#10911markdalgleish merged 18 commits intomainfrom
SuperHeaders class, add new from header methods#10911Conversation
MichaelDeBoey
left a comment
There was a problem hiding this comment.
@markdalgleish If this PR is created to fix #10872, an alternative could be to go with @alexandredev3's solution from #10889
|
@MichaelDeBoey Just repeating my comment here from #10913 for anyone who missed it. We were aware of this and explored what a complete solution would look like here: #10904. We've opted instead to simplify the model and avoid this category of issue entirely. |
|
Thanks for putting together this PR, @markdalgleish. I was thinking about this PR over the break, and it occurred to me that we could use Instead of exporting both the header class and a
We can also preserve the ability to pass an optional class AcceptEncoding {
static from(init: AcceptEncodingInit) {
let header = new AcceptEncoding()
// iterate over the init object and use set()/append() methods to add values...
return header
}
constructor(init?: AcceptEncodingInit) {
if (init) return AcceptEncoding.from(init)
this.#map = new Map()
}
}What do you think? |
|
@mjackson Yeah, that feels cleaner. I've pushed an update moving to this pattern. |
SuperHeaders class, add new parsing utilsSuperHeaders class, add new from header methods
mjackson
left a comment
There was a problem hiding this comment.
Almost there, just a few small tweaks.
Co-authored-by: Michael Jackson <michael@jackson.us>
|
I was using SuperHeaders to help construct responses - the API for which was very nice imo: return new Response(buffer, new SuperHeaders({
contentDisposition: { type: "attachment", filename },
contentType: "application/pdf",
cacheControl: { private: true, maxAge: 86400 }
})Is there a way to do this after these changes? If not, what do you think about adding a util that only builds a native Headers object without being a subclass, for example: return new Response(buffer, buildHeaders({
contentDisposition: { type: "attachment", filename },
contentType: "application/pdf",
cacheControl: { private: true, maxAge: 86400 }
})If you like the idea, I don't mind pushing a PR for review |
This PR removes the
SuperHeadersclass from@remix-run/headersin favour of individual header parsing functions and updates all consumers to use the new utils.@remix-run/headersno longer exportsSuperHeaders/Headersclass. Instead, it now exports individual.parse()methods for each header class (e.g.ContentType.parse(),Accept.parse(), etc.) along withparseRawHeaders()andstringifyRawHeaders()utilities.@remix-run/fetch-routernow returns a plainHeadersinstance fromRequestContext.headersinstead ofSuperHeaders, and has dropped the@remix-run/headerspeer dependency.@remix-run/multipart-parserhas been updated to use the new header parsing functions.@remix-run/responsehas been updated to use the new header parsing functions.